Fix ibis duckdb no rowid #5527
Fix ibis duckdb no rowid #5527
Conversation
| def test_datetime_xaxis(self): | ||
| """Test to make sure a DateTimeAxis can be identified for the bokeh backend""" | ||
| plot_ibis = Curve(self.reference_table, kdims="date", vdims="actual") | ||
| # When | ||
| plot_bokeh = render(plot_ibis, "bokeh") | ||
| xaxis, yaxis = plot_bokeh.axis | ||
| # Then | ||
| assert isinstance(xaxis, bokeh_axes.DatetimeAxis) | ||
| assert isinstance(yaxis, bokeh_axes.LinearAxis) | ||
|
|
||
| def test_categorical_xaxis(self): | ||
| """Test to make sure a Categorical axis can be identified for the bokeh backend""" | ||
| plot_ibis = Curve(self.reference_table, kdims="string", vdims="actual") | ||
| # When | ||
| plot_bokeh = render(plot_ibis, "bokeh") | ||
| xaxis, yaxis = plot_bokeh.axis | ||
| # Then | ||
| assert isinstance(xaxis, bokeh_axes.CategoricalAxis) | ||
| assert isinstance(yaxis, bokeh_axes.LinearAxis) | ||
|
|
||
| def test_numerical_xaxis(self): | ||
| """Test to make sure a LinearAxis axis can be identified for the bokeh backend""" | ||
| plot_ibis = Curve(self.reference_table, kdims="numerical", vdims="actual") | ||
| # When | ||
| plot_bokeh = render(plot_ibis, "bokeh") | ||
| xaxis, yaxis = plot_bokeh.axis | ||
| # Then | ||
| assert isinstance(xaxis, bokeh_axes.LinearAxis) | ||
| assert isinstance(yaxis, bokeh_axes.LinearAxis) |
There was a problem hiding this comment.
First of all, really appreciate your work investigating these issues. That said these tests can't live here. The plotting and data parts of HoloViews are separated for a reason and if test coverage of the data parts is sufficiently extensive then there's no reason to ever test the plotting code with different data backends.
There was a problem hiding this comment.
if test coverage of the data parts is sufficiently extensive
To be clear, that is of course clearly not the case since you're discovering these issues.
There was a problem hiding this comment.
Besides the non-comprehensive test, the missing docstrings and type annotations from the base Interface also makes it harder than it has to be, to know what a method on the interface actually should do and return.
There was a problem hiding this comment.
How would someone be able to setup appropriate tests for a backend @philippjfr? If I look into the tests/core/data folder I see nothing that systematically tests the same things across backends?
And knowing what to test for is also quite difficult due to missing docstrings, type annotations, very complex functions and similar.
There was a problem hiding this comment.
There are various baseclasses in tests/core/data which implement systematic tests which are then subclassed for each interface.
There was a problem hiding this comment.
The best thing to do is probably to continue extending tests/core/data/base HeterogeneousColumnTests with more tests.
There was a problem hiding this comment.
Specifically testing for datetime columns seems to be pretty lacking.
Codecov Report
@@ Coverage Diff @@
## master #5527 +/- ##
==========================================
- Coverage 88.07% 87.56% -0.51%
==========================================
Files 302 302
Lines 62283 62338 +55
==========================================
- Hits 54855 54589 -266
- Misses 7428 7749 +321
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
…x/ibis-duckdb-no-rowid
…x/ibis-duckdb-no-rowid
Closing #5526
Depends on #5522